fix(proxy): detect stalled upstream websockets - #1579

@codex review

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Reviewed watchdog lifecycle and settlement ownership. Transport side is sound: no custom watcher task, both watchdogs are library-managed, and the direct-WS send/receive settlement claim in websocket/mixin.py is airtight (synchronous claim, event set in finally, no await between claim and try-entry).

One settlement gap in the bridge reader blocks merge:

P1 — session.closed is an overloaded stand-in for liveness-settlement ownership. The break-without-settling guards in app/modules/proxy/_service/http_bridge/upstream_events.py (message path ~L966, exception path ~L1006) assume closed=True means a submitter owns the whole pending deque. But session.closed = True is written by ~30 sites, and some settle only their own request: e.g. the bridge_continuity_persistence_failed paths in request_submit.py (main ~L1114/L1130) set closed + retire_after_drain and leave sibling requests pending on a still-healthy socket. If that socket later heartbeat-expires, the reader hits the guard, breaks without settling the siblings, and exits the loop that enforced stream_idle_timeout — pre-PR the strand was bounded at ~600s by the idle timeout; post-PR it is unbounded until client disconnect. Repro: two requests pending on one bridge session, third submit fails record_recovery_attempt (closed=True, siblings untouched), then kill pongs — the liveness close settles nothing. Suggest a dedicated claim on the session (mirroring _WebSocketUpstreamControl.claim_liveness_settlement), set only by the liveness-owning submitter under lifecycle_lock, instead of keying on closed.

P3 — classification is pinned to library-internal strings (_AIOHTTP_HEARTBEAT_TIMEOUT_PREFIX = "No PONG received after ", sent-1011 + "keepalive ping timeout", proxy_websocket.py ~L82-86/L208-224). The new tests construct these exceptions themselves, so a dependency bump that rewords either library silently degrades liveness failures to upstream_unavailable — re-penalizing the account and dropping force-retire. One integration test driving a real keepalive expiry (short timeout against a fake server that never pongs) would pin the installed library's shape.

P3 — residual false-positive note, no change requested: both libraries stop processing pongs while transport reading is paused under recv backpressure, so a relay stalled >~ping_interval+timeout on downstream writes can kill a healthy upstream. Bounded by the same 120s downstream idle budget and the failure is account-neutral + retryable; the ops.md monitoring note covers it.

Heads-up: this rewrites the same settlement/admission-waiter region as #1594 and the reservation paths #1536 touches — whichever lands second will need a careful rebase. #1558 (finalizer settlement ordering) is merging ahead in this same territory; expect a rebase over its websocket/mixin.py finalizer changes.

@codex review

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Addressed in 5036851b after merging current main (including #1558) in 871addf7.

For the P1 settlement gap:

• _HTTPBridgeSession now has a dedicated liveness_settlement_owner claim. Production comments explicitly document that session.closed controls admission/retirement only and does not imply pending-deque settlement ownership.
• The submitter publishes the send claim synchronously inside the existing lifecycle_lock, immediately beside the classified liveness failure from send_text and before releasing that lock.
• Both bridge-reader guards now skip settlement only for that explicit send claim plus upstream_websocket_liveness_timeout; an unrelated closed=True no longer suppresses reader settlement.
• Added the exact regression shape from the review: two sibling requests remain pending, a third submit fails record_recovery_attempt and closes the session without a liveness claim, then the already-running reader receives the heartbeat expiry. The test verifies both siblings receive one terminal liveness failure, the deque and queue count drain, the account is not penalized, and forced retirement occurs.
• The pre-existing simultaneous send/receive liveness race test now also verifies that the submitter actually published the send claim and that settlement still occurs exactly once.

For the P3 library-shape coverage:

• Added an integration test that performs a real WebSocket handshake against a local raw server which reads frames but deliberately never answers pings.
• The installed websockets client runs with short real ping_interval / ping_timeout values; the production adapter must classify the library-generated close as upstream_websocket_liveness_timeout.
• The local connection uses proxy=None, following the local adversarial review, so host or CI proxy variables cannot divert the loopback test.

The backpressure false-positive note remains unchanged as requested; the existing account-neutral/retryable behavior and ops monitoring note still bound and document it.

Local verification after the #1558 merge:

• complete affected HTTP-bridge unit and WebSocket integration files: 554 passed
• focused ownership and real-watchdog tests under deliberately hostile proxy environment variables: 3 passed
• Ruff check + format, ty, and proxy architecture check: passed
• responses-api-compat OpenSpec strict validation: passed

GitHub CI for 5036851b is still in progress, so I am not claiming the cloud gate green yet.

@codex review

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@Soju06 please review

Full review plus two adversarial verification passes today — verdict: merge-ready in substance. The half-open detection gap is real (_RESPONSES_WEBSOCKET_POLICY disables both library watchdogs, leaving only the ~600s app-level idle backstops with an account-penalizing misclassification), the provenance-gated classification holds against the pinned library shapes (websockets 17.0.1 sent-first 1011, aiohttp 3.14.3 ServerTimeoutError), settlement ownership is exactly-once on the paths we traced, and the P1 from my earlier review is fixed exactly as suggested with the regression shape covered (test_http_bridge_closed_without_liveness_claim_still_settles_pending_siblings).

Two asks before merge:

1. Rebase — the branch went CONFLICTING after today's merges (fix(proxy): drain active WebSocket turns on shutdown #1520/fix(quota): release reservations on header failure #1559/fix(proxy): retry websocket EOF before visible output #1388-queue touch websocket/mixin.py, support.py, tests/unit/test_proxy_utils.py).
2. One P2 to fix in the same pass: cancellation between the sender-side liveness claim (http_bridge/request_submit.py ~L1168, latched under lifecycle_lock) and the whole-deque settlement in the outer except Exception handler (~L1254-1266) strands sibling requests — the claim latches permanently, settlement never runs, and the reader guards (upstream_events.py L969/L1014) then wait on a claim that will never settle, leaving siblings to the slower downstream backstop. Make the claim unable to outlive its settlement: run the claim→settlement region under a shield, or roll the claim back on CancelledError.

Two P3s are fine as follow-ups, noting them for the record: (a) the aiohttp heartbeat classification is pinned to the library-internal "No PONG received after " string with synthetic-only coverage (the real-handshake integration test covers only the websockets client); (b) peer-initiated keepalive ping timeout closes flip from account-neutral to penalizing under the new local-send-first provenance requirement — deliberate and spec-pinned, but worth an explicit line in the PR body.

Addressed in merge commit fdcc59b5.

Current-main integration

• Merged origin/main at 410bfaea as the second parent (no force-push), resolving the overlaps in websocket/mixin.py, support.py, the bridge helpers, and the direct-WebSocket tests.
• Kept current main's newer direct-WebSocket ownership model: a transport-end message is finalized by a registered, shielded child task, while send failure quiesces that reader before taking the remaining queue. This PR's liveness behavior is applied inside that model: account-neutral classification, no ambiguous replay, sequenced downstream suppression, and transport retirement.
• Updated the two newly merged routed-connect provenance test settings stubs with the existing idle-timeout field required by the liveness policy.

P2 — cancellation after the HTTP bridge sender claim

• The sender still publishes liveness_settlement_owner = "send" synchronously beside the failed send_text, while holding session.lifecycle_lock; closed remains admission/retirement state only.
• The outer liveness handler now publishes http-bridge-liveness-send-settlement, a dedicated child that reacquires lifecycle_lock and performs _fail_http_bridge_reader_and_maybe_retire(..., force_retire=True) for the whole deque.
• _await_task_deferring_cancellation shields that child. If caller cancellation arrives at the first await after the claim, the child continues through all terminal events, reservation/gate release, deque drain, and retirement. The original CancelledError is re-raised only after the child completes.
• Production comments at the claim and settlement sites state why the task must be published before the first post-claim await, so future refactors do not reopen the gap.

The existing simultaneous send/receive liveness-race regression now runs in two variants: normal and cancelled-after-claim. The cancellation variant pauses the whole-deque finalizer, cancels the submitter after verifying the send claim, verifies the submitter remains alive while settlement is blocked, then releases settlement and checks:

• both the older sibling and current request receive exactly one liveness terminal plus queue sentinel;
• the pending deque and queued count drain to zero;
• the reader does not replay or double-settle;
• account health remains neutral and forced retirement runs once;
• caller cancellation is preserved after settlement completes.

P3 record

• The aiohttp real-handshake coverage remains a follow-up as noted.
• The PR body now explicitly states the provenance boundary: only a locally sent watchdog 1011 with keepalive ping timeout is account-neutral; a peer-initiated close with the same code/reason remains an ordinary account-penalizing failure.

Local verification on fdcc59b5:

• affected unit suites: 1519 passed
• direct-WebSocket integration: 110 passed
• HTTP bridge integration: 121 passed
• Ruff, format, changed-file ty, proxy architecture: passed
• strict responses-api-compat and full spec validation: 49 passed, 0 failed

Repository-wide ty currently reports one unrelated diagnostic inherited unchanged from origin/main in tests/integration/test_db_commit_durability.py (update_last_used); the changed-file type check is clean. Cloud CI for the pushed merge head is starting, so I am not claiming the GitHub gate green yet.

@Soju06 please review

@codex review

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: c98bc04622